Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added ability to use library in webbrowser with requiring stopwords files #84

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ageitgey
Copy link
Owner

@ageitgey ageitgey commented Apr 4, 2018

This is a PR of a commit from @cjanietz

@cjanietz are you ok with me pulling this in?

@cjanietz
Copy link

Go ahead, fine for me

getFilePath = (language) ->
path.join(__dirname, "..", "data", "stopwords", "stopwords-#{language}.txt")

# Given a language, loads a list of stop words for that language
# and then returns which of those words exist in the given content
module.exports = stopwords = (content, language = 'en') ->
filePath = getFilePath(language)
hasFs = 'existsSync' in fs
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest this line should be hasFs = fs and fs.existsSync.

The current line fails because the compiled JavaScript line treats fs as an array with length, and fs.length does not exist:

function in$(member, list) {
  for (var i = 0, length = list.length; i < length; ++i) // <-- failure at list.length
    if (i in list && list[i] === member)
      return true;
  return false;
}
hasFs = in$('existsSync', fs);

The proposed change succeeds in JavaScript as hasFs = fs && fs.existsSync. This should fix the buildbots.

I would PR to the branch myself, but I'm unfamiliar with CoffeeScript and can't build.

@justinmchase
Copy link

justinmchase commented Nov 27, 2021

Maybe drop fs entirely and convert the stop word files to code:

# en.coffee
module.exports = [
  "a's",
  "able",
  # ... etc.
]
stopwords = require(language)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants